Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(s3-deployment): exclude and include filters #16054

Merged
merged 13 commits into from
Aug 19, 2021
Merged

Conversation

davidtucker
Copy link
Contributor

@davidtucker davidtucker commented Aug 14, 2021

This construct only enables integration with the two existing CLI options (--exclude and --include) that are supported for the s3 sync command. There are a few situations where this can prove valuable:

  1. Situations where you want to deploy a subset of files from an archive - This can be handled by leveraging the bundling option for a source, although in some situations the exclude filter would be significantly easier.
  2. Situations where you want to leverage prune but have specific files excluded - This is the situation that cannot be solved with current tools. The most common scenario (and one I detailed in (aws-s3-deployment): Support include and exclude options for S3 sync #14362 ) is where you manage a web app config file with a custom CloudFormation resource (to pass in API endpoint, user pool, etc...) and then manage a web application using this construct.

Closes #14362


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@gitpod-io
Copy link

gitpod-io bot commented Aug 14, 2021

@eladb eladb changed the title feat(aws-s3-deployment): enable exclude and include filters feat(s3-deployment): exclude and include filters Aug 17, 2021
eladb
eladb previously requested changes Aug 17, 2021
Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Please change API to support more than a single exclude/include pattern

@mergify mergify bot dismissed eladb’s stale review August 17, 2021 09:24

Pull request has been modified.

eladb
eladb previously approved these changes Aug 17, 2021
@mergify mergify bot dismissed eladb’s stale review August 17, 2021 11:16

Pull request has been modified.

@davidtucker
Copy link
Contributor Author

@eladb I just realized I missed the README update, but I see that you already tackled that. Apologies for not catching that in the update.

eladb
eladb previously requested changes Aug 17, 2021
Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized something - AssetOptions also have support for exclusions, which are also taken into account when calculating the asset hash (and therefore invalidate the asset appropriately). @davidtucker I am wondering if using exclusions at the asset level is sufficient and addresses your use case. What do we gain from adding this at the deployment time?

@davidtucker
Copy link
Contributor Author

@eladb The main issue here (that led to me creating this PR) is that I want to exclude files from being pruned in the destination bucket. In my original use case, I have two things in a bucket:

  • A web application (uploaded using aws-s3-deployment)
  • A config file (creating using a CloudFormation custom resource)

Using the S3 CLI, this is east to manage. You can set it to prune with --delete but exclude the config file. You can see further discussion on this in #14362 . If there is another way to address this, I am fine with it, but I think just using AssetOptions solves one issue (including and excluding at the asset level) but not the other (excluding files in the destination bucket from the sync process).

@eladb
Copy link
Contributor

eladb commented Aug 19, 2021

Using the S3 CLI, this is east to manage. You can set it to prune with --delete but exclude the config file. You can see further discussion on this in #14362 . If there is another way to address this, I am fine with it, but I think just using AssetOptions solves one issue (including and excluding at the asset level) but not the other (excluding files in the destination bucket from the sync process).

OK, makes sense. Can you please add a note in the docs for include and exclude that points users to the asset exclude option if they want to exclude files from being evaluated when invalidating the asset?

@mergify mergify bot dismissed eladb’s stale review August 19, 2021 13:10

Pull request has been modified.

@eladb
Copy link
Contributor

eladb commented Aug 19, 2021

Thanks for the awesome contribution @davidtucker

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: f558345
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit d42e89e into aws:master Aug 19, 2021
@mergify
Copy link
Contributor

mergify bot commented Aug 19, 2021

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

smguggen pushed a commit to smguggen/aws-cdk that referenced this pull request Aug 24, 2021
This construct only enables integration with the two existing CLI options (`--exclude` and `--include`) that are supported for the `s3 sync` command. There are a few situations where this can prove valuable:

1. Situations where you want to deploy a subset of files from an archive - This can be handled by leveraging the bundling option for a source, although in some situations the `exclude` filter would be significantly easier.
2. Situations where you want to leverage `prune` but have specific files excluded - This is the situation that cannot be solved with current tools.  The most common scenario (and one I detailed in aws#14362 ) is where you manage a web app config file with a custom CloudFormation resource (to pass in API endpoint, user pool, etc...) and then manage a web application using this construct.  

Closes aws#14362 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this pull request Aug 26, 2021
This construct only enables integration with the two existing CLI options (`--exclude` and `--include`) that are supported for the `s3 sync` command. There are a few situations where this can prove valuable:

1. Situations where you want to deploy a subset of files from an archive - This can be handled by leveraging the bundling option for a source, although in some situations the `exclude` filter would be significantly easier.
2. Situations where you want to leverage `prune` but have specific files excluded - This is the situation that cannot be solved with current tools.  The most common scenario (and one I detailed in aws#14362 ) is where you manage a web app config file with a custom CloudFormation resource (to pass in API endpoint, user pool, etc...) and then manage a web application using this construct.  

Closes aws#14362 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this pull request Sep 6, 2021
This construct only enables integration with the two existing CLI options (`--exclude` and `--include`) that are supported for the `s3 sync` command. There are a few situations where this can prove valuable:

1. Situations where you want to deploy a subset of files from an archive - This can be handled by leveraging the bundling option for a source, although in some situations the `exclude` filter would be significantly easier.
2. Situations where you want to leverage `prune` but have specific files excluded - This is the situation that cannot be solved with current tools.  The most common scenario (and one I detailed in aws#14362 ) is where you manage a web app config file with a custom CloudFormation resource (to pass in API endpoint, user pool, etc...) and then manage a web application using this construct.  

Closes aws#14362 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
david-doyle-as24 pushed a commit to david-doyle-as24/aws-cdk that referenced this pull request Sep 7, 2021
This construct only enables integration with the two existing CLI options (`--exclude` and `--include`) that are supported for the `s3 sync` command. There are a few situations where this can prove valuable:

1. Situations where you want to deploy a subset of files from an archive - This can be handled by leveraging the bundling option for a source, although in some situations the `exclude` filter would be significantly easier.
2. Situations where you want to leverage `prune` but have specific files excluded - This is the situation that cannot be solved with current tools.  The most common scenario (and one I detailed in aws#14362 ) is where you manage a web app config file with a custom CloudFormation resource (to pass in API endpoint, user pool, etc...) and then manage a web application using this construct.  

Closes aws#14362 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(aws-s3-deployment): Support include and exclude options for S3 sync
3 participants